fix: fix file locking error when removing cache folder #162
Conversation
Summary of ChangesHello @ChenZiHong-Gavin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses cache-related errors by improving resource management and making the temporary workspace cleanup process more robust. It ensures that Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix a cache error by improving resource cleanup. In webui/app.py, it introduces manual garbage collection for the Engine object. In webui/utils/cache.py, it makes the workspace cleanup more robust by handling errors during file deletion.
My review focuses on making these fixes more robust.
- I've suggested explicitly terminating Ray actors in
app.pyto prevent resource leaks, asdelandgc.collect()alone are not sufficient. - In
cache.py, I've pointed out that silently ignoring cleanup errors can hide persistent issues and suggested a safer way to modify file permissions to avoid unintended side effects.
Overall, the changes are in the right direction, but the suggested improvements will make the resource management more reliable.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Silently swallowing exceptions with except Exception: pass can hide underlying issues and make debugging difficult. If the cleanup consistently fails for some reason, you would have no way of knowing, and temporary files could accumulate. Consider logging the exception here, even if you don't re-raise it, so that persistent problems can be identified.
ChenZiHong-Gavin
changed the title
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| time.sleep(0.5) | ||
| try: | ||
| func(path) | ||
| except Exception: |
This PR addresses cache-related errors by improving resource management and making the temporary workspace cleanup process more robust.